-
Notifications
You must be signed in to change notification settings - Fork 901
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix ANR caused by Duck Player #5169
Conversation
This stack of pull requests is managed by Graphite. Learn more about stacking. Join @CrisBarreiro and the rest of your teammates on Graphite |
89679f7
to
e71ec4d
Compare
val willNavigateToDuckPlayerDeferred = scope.async(dispatcherProvider.io()) { duckPlayer.willNavigateToDuckPlayer(uri) } | ||
|
||
val willNavigateToDuckPlayer = runBlocking(dispatcherProvider.io()) { willNavigateToDuckPlayerDeferred.await() } | ||
val willNavigateToDuckPlayer = runBlocking(dispatcherProvider.io()) { duckPlayer.willNavigateToDuckPlayer(uri) } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice!
@@ -371,6 +373,7 @@ class RealDuckPlayer @Inject constructor( | |||
} | |||
|
|||
private suspend fun doesYoutubeUrlComeFromDuckPlayer(url: Uri, request: WebResourceRequest? = null): Boolean { | |||
val start = SystemClock.elapsedRealtime() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this needed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tested and works as expected! ✅
Just the one comment: #5169 (comment)
e71ec4d
to
71012d6
Compare
71012d6
to
085784e
Compare
Task/Issue URL: https://app.asana.com/0/1202552961248957/1208580918293550/f
Description
DuckPlayerFeatureRepository
, as it's no longer needed and we can rely on StateFlows insteadSteps to test this PR
Feature 1